Add support for Scala 3 crossbuilds, raise min version to 1.5.8 #256
Conversation
|
There is #255 |
|
This one looks better than #255, so let's continue here |
| // in order to enable colors we trick git into thinking we're a pager, because it already knows we're not a tty | ||
| val colorSupport: Seq[(String, String)] = | ||
| if(ConsoleLogger.formatEnabled) Seq("GIT_PAGER_IN_USE" -> "1") | ||
| if(Terminal.console.isAnsiSupported) Seq("GIT_PAGER_IN_USE" -> "1") |
There was a problem hiding this comment.
Terminal.console can throw an IllegalStateException here so it should be wrapped with try catch.
There was a problem hiding this comment.
I used monadic Try, lemme know if you prefer try/catch for some reason. There's a few other places where Try is used in the plugin, but it's certainly not uniform.
| private def branchesRef: Seq[Ref] = { | ||
| import collection.JavaConverters._ | ||
| porcelain.branchList.call.asScala | ||
| import scala.jdk.CollectionConverters._ |
There was a problem hiding this comment.
This is repeated 6 times. I suggest to move the import to the top of the file.
There was a problem hiding this comment.
Sure, I'll get right on that.
I was thinking the same thing as I was making this conversion. I didn't do this because I haven't worked on the internals here, and wasn't sure if this was a runtime optimization or not.
5 participants
Towards #257
1.5.8for1.xcrossbuilds and scala crossbuilds+ compileand+ testall work successfully. However^testruns into a cyclical error. Since we're using 2/3 crossbuilding everywhere else (with matrices), I opted to remove the old crossbuild for now. Will add the 2.0 + scala3 matrix to CI later.